feat(tables): background jobs (delete/export/backfill on trigger.dev) + tenant-scoped query performance#4915
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview New async APIs: Bulk table UX: Select-all row selection supports exclusions; delete/run/stop/cancel accept filter + Ops: Stale-job cron marks stalled Reviewed by Cursor Bugbot for commit 4fc09f7. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR ships background-job infrastructure for Tables: a new
Confidence Score: 5/5Safe to merge — the concurrency gate, delete mask, keyset paging, and ghost-job fixes all look correct; the findings are optimization opportunities rather than blockers. The export runner buffers the entire serialized file in memory before uploading, and pendingDeleteMask is re-queried on every export page rather than once per job. Both are optimization opportunities with no risk to data correctness or user-visible state, and the 1 M-row manual test suggests they work within practical limits for today's workloads. apps/sim/lib/table/export-runner.ts — in-memory file accumulation will need attention before this export path is stressed with very wide or extremely large tables. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Route as POST /delete-async
participant Service as table service
participant DB as PostgreSQL
participant Worker as delete runner
participant SSE as SSE stream
Client->>Route: "{filter, excludeRowIds, estimatedCount}"
Route->>Service: markTableJobRunning(tableId, jobId, 'delete', payload)
Service->>DB: INSERT table_jobs ON CONFLICT DO NOTHING
DB-->>Service: "claimed = true"
Route->>Worker: trigger table-delete task
Note over Route,Worker: on trigger failure to releaseJobClaim to throw
Route-->>Client: "{jobId}"
loop keyset pages
Worker->>DB: updateJobProgress (heartbeat + ownership)
DB-->>Worker: "owns = true"
Worker->>DB: selectRowIdPage (table_id+id cursor, cutoff, filter)
DB-->>Worker: page ids
Worker->>DB: deletePageByIds
Worker->>SSE: appendTableEvent(running, progress)
end
Worker->>DB: markJobReady(tableId, jobId)
Worker->>SSE: appendTableEvent(ready, total)
Note over Client,SSE: Grid reads use pendingDeleteMask to hide doomed rows
Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/sta..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ed optimistic clear, Cmd+A select-all, hide delete from tray)
…row-deletes # Conflicts: # apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx # apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx # apps/sim/app/workspace/[workspaceId]/tables/tables.tsx # apps/sim/lib/table/import-runner.ts # apps/sim/lib/table/service.ts # packages/db/migrations/meta/0227_snapshot.json # packages/db/migrations/meta/_journal.json
…sk, keyset index + autovacuum tuning
…ith in-process fallback
…ancel, and download
…instead of the import tray
… toasts instead of the import tray" This reverts commit 1ea5871.
…n one spinner tray
Select-all minus deselected rows now means exactly that for every bulk action, not just delete. runColumnBodySchema and cancelTableRunsBodySchema accept excludeRowIds (bounded by MAX_EXCLUDE_ROW_IDS, select-all scope only); the dispatch scope persists it and the dispatcher window walk, eager bulk-clear, pre-run cancel, and filter/table-scoped cancel all skip excluded rows. Client threads exclusions from the selection through the action bar and the grid context menu, including the optimistic stamps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lder table Two Bugbot findings on the exclusion work: - Select-all-minus-deselections Stop (no filter) cancelled every active dispatch table-wide, killing row-scoped runs on deselected rows. markActiveDispatchesCancelled now spares dispatches whose scope.rowIds are fully contained in the exclusion set (coalesce(false) keeps table-wide dispatches cancellable). - Create-mode import: a failed trigger.dev dispatch released the job claim but left the just-created placeholder table in the workspace. Archive it on the failure path (no hard-delete surface exists). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…row-deletes Migration renumbered 0231 -> 0232 (staging took 0231) and reworked for the new CI-driven migration runner: - expand/contract: the import_* column drops are removed — the previous app version still reads them while migrations apply from CI; a later release drops them (data migration gains ON CONFLICT DO NOTHING and the whole file is replay-idempotent per the runner convention) - user_table_rows indexes (keyset btree + tenant-scoped GIN) build via COMMIT + CREATE INDEX CONCURRENTLY so the write-hot shared relation is never write-blocked; old GIN dropped CONCURRENTLY after the new one Also unions the rows-route imports with staging's row-wire translators and re-baselines the route count (811 staging + 2 export routes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A mid-delete refresh resurrected the old counts: the optimistic update stripped cached rows but left page-0 totalCount (footer / select-all label) at the old total, and list/detail counts reported raw row_count including doomed-but-not-yet-deleted rows. - onMutate now sets the active view's totalCount to the kept rows and decrements the cached detail rowCount by the doomed estimate - the kickoff persists that estimate on the job (payload.doomedCount, clamped server-side); getTableById/listTables subtract the not-yet-deleted remainder (doomedCount - rows_processed) while the delete runs, so refetched counts match the read path's delete mask Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| toast.success('Export started — the download will begin when it finishes') | ||
| } else { | ||
| await downloadTableExport(tableData.id, tableData.name) | ||
| } |
There was a problem hiding this comment.
Export callback missing hook dependency
Low Severity
handleExportCsv calls exportTableAsync.mutateAsync but its useCallback dependencies list only tableData and workspaceId, not exportTableAsync (from useExportTableAsync declared later). After navigation or hook identity changes, export can run against a stale mutation bound to the wrong table.
Reviewed by Cursor Bugbot for commit 4118766. Configure here.
There was a problem hiding this comment.
False positive per repo convention (.claude/rules/sim-queries.md): TanStack v5 mutateAsync is stable and always invokes the current mutationFn — the mutation observer reads the latest options on each call, so a memoized closure over an older mutation object still targets the current tableId. The rules explicitly say not to add mutation objects to useCallback deps. Additionally tableData is in the deps and changes identity on table navigation, refreshing the closure anyway.
The header checkbox lingered as a minus over the optimistically-emptied grid: rowSelectionCoversAll treats zero rows as not-covered, and the selection clear waited for the kickoff's onSuccess. Clear at click (failed kickoffs visibly restore rows + toast; re-selecting is cheap) and render an empty grid's header checkbox unchecked regardless — a selection over zero rows is vacuous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sync/async export choice reads rowCount, which is a doomed-estimate- adjusted number during a running delete (and the estimate is client- supplied) — an overstated estimate could route a still-large masked set through the synchronous stream. Mid-delete exports now always run as a job: safe at any size, and exports bypass the one-job-per-table gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…row-deletes Migration renumbered 0232 -> 0233 (staging took 0232 for BYOK keys); snapshot regenerated, hand-written SQL preserved, zero drift. checkUniqueConstraintsDb reconciles staging's executor param (pool self-deadlock fix #4975) with the tenant-bounded planner flag: own transaction only when given plain db, SET LOCAL on the caller's transaction otherwise. process-contents test keeps relying on global mocks (now incl. dbReplica). Route baseline 815 (+2 staging tools). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…raphs next build (Turbopack) failed with "Two or more assets with different content were emitted to the same output path" on the server-root chunk. Root cause: setup.server.ts's unscoped path.resolve(process.cwd()) made node-file-tracing sweep the entire project — next.config.ts included — into every route graph reaching lib/uploads (the files/upload route and, since the export job, the export-async path). Two producers emitted the swept config into same-named chunks; staging's latest commits made their contents diverge and the names collided. Annotate the path derivation with turbopackIgnore per the NFT warning's own remediation — the build passes and all ~390 "unexpected file in NFT list" warnings disappear. Also inline the releaseJobClaim dynamic imports in the kickoff routes to plain static imports — service is already statically imported there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptile review |
…row-deletes Route baseline 816 (integrations' routes + our two export routes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4fc09f7. Configure here.
1 participant
Summary
Background jobs, async exports, and a systemic performance pass for Tables.
Background row deletes +
table_jobs{ filter, excludeRowIds }and delete in keyset-paginated background batches; rows inserted mid-job are spared via acreated_atcutoff.table_jobstable (one row per job,type= import | delete | export | backfill). Concurrency gate: partial-unique index on(table_id) WHERE status='running' AND type <> 'export'— one writing job per table; read-only exports run concurrently.Jobs on trigger.dev
apps/sim/background/) with an in-processrunDetachedfallback — jobs survive deploys/restarts.CSV export
Performance (measured on a 1M-row table in a 12M-row shared relation)
(table_id, id)index (0231)(order_key, id)cursor seek$inRoot cause for most of these: jsonb predicates are unestimatable, so the planner picked parallel seq scans over the entire shared relation (every tenant's rows). Two mechanisms fix it:
withSeqscanOff(lib/table/planner.ts): transaction-scopedSET LOCAL enable_seqscan = offaround the queries with unindexable predicates (ILIKE/range/lower()/sort keys).datawithbtree_gin (table_id, data jsonb_path_ops)— containment lookups resolve tenant-scoped inside the index (and the index is smaller, 529MB vs 694MB on dev).Fit & finish
Migrations
0231_table_jobs_and_keyset:table_jobs+ import-state data migration +(table_id, id)index + autovacuum tuning +import_*column drops.0232_tenant_scoped_data_gin:btree_ginextension + tenant-scoped containment index (replacesuser_table_rows_data_gin_idx).Test plan
bunx vitest run lib/table app/api/table app/api/v1 hooks/queries(395 tests)bun run lint:check,tsc --noEmit,bun run check:api-validationTRIGGER_DEV_ENABLED=true; EXPLAIN ANALYZE before/after for each perf claim.🤖 Generated with Claude Code